-
Notifications
You must be signed in to change notification settings - Fork 0
[Refactor] Response Handler 적용 #224 #235
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
- HealthService의 getDailyHealth 함수 반환 타입을 `Response<HealthResponseDto>`로 수정 - Ktorfit의 `Response`를 사용하여 API 호출의 성공/실패 여부를 명확하게 처리하도록 변경
|
Caution Review failedThe pull request is closed. Walkthrough데이터 계층에 Retrofit Changes
Sequence Diagram(s)(생성 조건 미충족 — 생략) Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Suggested labels
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 5
🤖 Fix all issues with AI agents
In
`@app/src/main/java/com/konkuk/medicarecall/data/repositoryimpl/EldersHealthInfoRepositoryImpl.kt`:
- Around line 45-51: The call to elderRegisterService.postElderHealthInfo(...)
uses handleResponse(), which fails because postElderHealthInfo returns
Response<Unit> (204 No Content with null body); replace the call to
handleResponse() with handleNullableResponse() so a null body is treated as a
successful response, i.e. update the invocation on the result of
elderRegisterService.postElderHealthInfo(elderInfo.elderId, elder) to use
handleNullableResponse(), keeping the surrounding code
(ElderHealthRegisterRequestDto construction and refresh() call) unchanged.
In
`@app/src/main/java/com/konkuk/medicarecall/data/repositoryimpl/SetCallRepositoryImpl.kt`:
- Around line 15-20: saveForElder is calling
service.saveCareCallTimes(...).handleResponse(), which throws on 204 No Content
because ResponseHandler expects a non-null body; change the call to use
handleNullableResponse() instead (i.e., replace handleResponse() with
handleNullableResponse() in saveForElder) or update the ResponseHandler to treat
Unit/empty bodies as success (special-case when T is Unit or
response.code()==204) so a null body doesn't raise error; specifically look for
saveForElder, service.saveCareCallTimes, handleResponse, handleNullableResponse
and adjust the call or ResponseHandler logic accordingly.
In `@app/src/main/java/com/konkuk/medicarecall/data/util/ResponseHandler.kt`:
- Around line 6-11: handleResponse() currently throws when body() is null, which
breaks endpoints that return Response<Unit> on 204 No Content; update
handleResponse (or replace calls with handleNullableResponse) to treat null body
as valid when the response code is 204 or when T is Unit: in the success branch,
if body() != null return it, otherwise if (code() == 204 || T represents
kotlin.Unit) return Unit cast to T, else throw the existing error; apply this
change in the handleResponse function used by
SetCallService.saveCareCallTimes(), HomeService.requestImmediateCareCall(),
EldersInfoService.deleteElderSettings(), ElderRegisterService methods,
AuthService.requestCertificationCode()/logout(), and FcmService methods (or
switch those callers to handleNullableResponse()).
In `@REFACTORING_ANALYSIS.md`:
- Around line 41-62: The markdown code fences in REFACTORING_ANALYSIS.md are
missing language identifiers (violates MD040); update each fenced block (e.g.,
the project tree blocks shown in the diff) to add an appropriate language tag
such as ```text or ```kotlin/```gradle where applicable, ensuring opening fences
include the language and closing fences remain ```; apply the same change to the
other occurrences called out in the comment (around the blocks at lines ~71-74,
~171-181, and ~282-286) so all fenced code blocks have language specifiers.
- Around line 26-33: The Markdown table block lacks surrounding blank lines,
violating MD058; add one empty line before the table start ("| 항목 | 수치 |") and
one empty line after the table end ("| 테스트 코드 | 3개 (극히 부족) |") so the table is
separated from surrounding paragraphs and complies with markdownlint.
app/src/main/java/com/konkuk/medicarecall/data/repositoryimpl/EldersHealthInfoRepositoryImpl.kt
Show resolved
Hide resolved
app/src/main/java/com/konkuk/medicarecall/data/repositoryimpl/SetCallRepositoryImpl.kt
Show resolved
Hide resolved
- `HealthRepositoryImpl`에서 `handleResponse()` 유틸 함수를 사용하여 API 응답을 처리하도록 수정 - 코드 중복을 줄이고 일관성 있는 오류 처리 방식 적용
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
app/src/main/java/com/konkuk/medicarecall/data/repositoryimpl/HealthRepositoryImpl.kt (1)
18-20:symptomList에!!사용 시 NPE 가능성 있어요Line 18에서는
orEmpty()로 null을 안전하게 처리하는데, Line 20에서는 같은 필드에!!를 사용하고 있어서 일관성이 없고symptomList가 null일 경우 NPE가 발생합니다.🔧 제안하는 수정
HealthUiState( symptoms = response.symptomList.orEmpty(), symptomAnalysis = response.analysisComment.orEmpty(), - isRecorded = response.symptomList!!.isNotEmpty() || !response.analysisComment.isNullOrBlank(), + isRecorded = !response.symptomList.isNullOrEmpty() || !response.analysisComment.isNullOrBlank(), )
- `handleResponse`를 `handleNullableResponse`로 변경하여 Unit을 반환하는 API 호출 처리
- 적용된 Repository
- EldersInfoRepositoryImpl
- UserRepositoryImpl
- ElderRegisterRepositoryImpl
- UpdateElderInfoRepositoryImpl
- HomeRepositoryImpl
- EldersHealthInfoRepositoryImpl
- SetCallRepositoryImpl
- `REFACTORING_ANALYSIS.md` 파일 삭제 - 프로젝트 리팩토링을 위한 분석 내용이 포함되어 있었음
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 2
🤖 Fix all issues with AI agents
In
`@app/src/main/java/com/konkuk/medicarecall/data/repositoryimpl/HomeRepositoryImpl.kt`:
- Around line 24-25: getHomeSummary currently returns HomeResponseDto directly
from HomeRepositoryImpl.getHomeSummary by calling
homeService.getHomeSummary(elderId).handleResponse(), which breaks consistency
with other repository methods and can propagate exceptions; update
HomeRepository and HomeRepositoryImpl.getHomeSummary to return
Result<HomeResponseDto> and wrap the service call in runCatching {
homeService.getHomeSummary(elderId).handleResponse() } so callers receive a
Result and exceptions are captured instead of thrown.
In
`@app/src/main/java/com/konkuk/medicarecall/data/repositoryimpl/UserRepositoryImpl.kt`:
- Around line 18-24: updateMyInfo in UserRepositoryImpl currently calls
settingService.updateMyInfo(...).handleResponse(), but if the server returns
204/empty body this will cause handleResponse() to throw a "response is null"
error; check the actual SettingService.updateMyInfo API contract and if it can
return an empty body, replace handleResponse() with handleNullableResponse() for
updateMyInfo in UserRepositoryImpl (and update the comment near SettingService
line 18 if needed) so the nullable/empty response is handled safely while
leaving getMyInfo() unchanged.
🧹 Nitpick comments (1)
app/src/main/java/com/konkuk/medicarecall/data/repositoryimpl/EldersInfoRepositoryImpl.kt (1)
41-42:!!연산자 사용 시 NPE 가능성
find { ... }!!패턴은 매칭되는 항목이 없으면 NPE가 발생합니다. 물론 UI에서 유효한 값만 넘어온다는 전제가 있겠지만, 방어적으로 처리하면 더 안전할 것 같아요.♻️ 제안: firstOrNull + require 또는 예외 메시지 추가
- relationship = RelationshipType.entries.find { it.displayName == request.relationship }!!, - residenceType = ElderResidenceType.entries.find { it.displayName == request.livingType }!!, + relationship = requireNotNull( + RelationshipType.entries.find { it.displayName == request.relationship } + ) { "Unknown relationship: ${request.relationship}" }, + residenceType = requireNotNull( + ElderResidenceType.entries.find { it.displayName == request.livingType } + ) { "Unknown residenceType: ${request.livingType}" },
app/src/main/java/com/konkuk/medicarecall/data/repositoryimpl/HomeRepositoryImpl.kt
Outdated
Show resolved
Hide resolved
| override suspend fun getMyInfo() = runCatching { | ||
| val response = settingService.getMyInfo() | ||
| if (response.isSuccessful) { | ||
| response.body() ?: error("Response body is null") | ||
| } else { | ||
| val errorBody = response.errorBody()?.toString() ?: "Unknown error" | ||
| throw HttpException(response) | ||
| } | ||
| settingService.getMyInfo().handleResponse() | ||
| } | ||
|
|
||
| override suspend fun updateMyInfo(userUpdateRequestDto: MyInfoResponseDto) = runCatching { | ||
| Log.d("UserRepository", "updateMyInfo() 진입: $userUpdateRequestDto") | ||
| val response = settingService.updateMyInfo(userUpdateRequestDto) | ||
| if (response.isSuccessful) { | ||
| response.body() ?: error("Response body is null") | ||
| } else { | ||
| throw HttpException(response) | ||
| } | ||
| settingService.updateMyInfo(userUpdateRequestDto).handleResponse() | ||
| } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🧩 Analysis chain
🏁 Script executed:
rg -n "interface\s+SettingService|fun\s+updateMyInfo\b" -C 3 --type=ktRepository: Medicare-Call/Medicare-Call-Android
Length of output: 105
🏁 Script executed:
# Find handleResponse and handleNullableResponse implementations
rg -n "fun\s+(handleResponse|handleNullableResponse)" --type=kt -C 2Repository: Medicare-Call/Medicare-Call-Android
Length of output: 105
🏁 Script executed:
# Find SettingService interface file
fd -t f "SettingService" --type ktRepository: Medicare-Call/Medicare-Call-Android
Length of output: 248
🏁 Script executed:
# Find SettingService interface and updateMyInfo method
rg -n "interface\s+SettingService|fun\s+updateMyInfo\b" -g "*.kt" -C 3Repository: Medicare-Call/Medicare-Call-Android
Length of output: 3096
🏁 Script executed:
# Find handleResponse and handleNullableResponse implementations
rg -n "fun\s+(handleResponse|handleNullableResponse)" -g "*.kt" -C 2Repository: Medicare-Call/Medicare-Call-Android
Length of output: 61
🏁 Script executed:
# List Kotlin files related to SettingService
fd "SettingService" -e ktRepository: Medicare-Call/Medicare-Call-Android
Length of output: 154
🏁 Script executed:
# Search for handleResponse and handleNullableResponse definitions more broadly
rg -n "handleResponse|handleNullableResponse" -g "*.kt" -B 2 -A 5Repository: Medicare-Call/Medicare-Call-Android
Length of output: 37513
SettingService.updateMyInfo 응답 타입 확인 필요
updateMyInfo가 204/빈 바디 응답을 반환하면 handleResponse()가 null 체크에서 "응답값이 null 입니다." 에러를 던질 수 있습니다. SettingService의 실제 서버 스펙을 확인하고 필요시 handleNullableResponse()로 변경해주세요. (SettingService 라인 18의 주석도 참고)
🤖 Prompt for AI Agents
In
`@app/src/main/java/com/konkuk/medicarecall/data/repositoryimpl/UserRepositoryImpl.kt`
around lines 18 - 24, updateMyInfo in UserRepositoryImpl currently calls
settingService.updateMyInfo(...).handleResponse(), but if the server returns
204/empty body this will cause handleResponse() to throw a "response is null"
error; check the actual SettingService.updateMyInfo API contract and if it can
return an empty body, replace handleResponse() with handleNullableResponse() for
updateMyInfo in UserRepositoryImpl (and update the comment near SettingService
line 18 if needed) so the nullable/empty response is handled safely while
leaving getMyInfo() unchanged.
librawish808
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
좋습니다
- runCatching 블록 내에서 결과를 직접 반환하도록 수정 - 불필요한 `result` 변수 제거
- `HomeRepository.getHomeSummary` 함수의 반환 타입을 `Result`로 변경하여 API 호출 실패 시 예외를 처리하도록 수정 - `HomeViewModel`에서 `getOrThrow()`를 사용하여 `Result` 객체에서 데이터를 추출하고, 실패 시 예외를 발생시키도록 변경
✏️ 변경사항
📷 스크린샷
N/A
✍️ 사용법
🎸 기타
Summary by CodeRabbit
✏️ Tip: You can customize this high-level summary in your review settings.